[v0.35] fix: CRD sync race in vcluster HA (#3993)#4002
Conversation
a809623 to
2a7418c
Compare
E2E Ginkgo Tests
|
(cherry picked from commit 153381c)
2a7418c to
3d228d7
Compare
sowmyav27
left a comment
There was a problem hiding this comment.
Approving — the fix is correct and the core scenario is covered (first CRD update hits a conflict → retried → succeeds, no crash-loop).
Test coverage note (non-blocking — fine as a follow-up PR, not a blocker for this backport):
-
The already-converged skip path isn't exercised. The GET stub returns the same un-updated CRD on every call, so
apiequality.Semantic.DeepEqual(current, desired)is always false and thereturn nilearly-return (skip the update when a peer replica already added the version) never runs — even though the test name says "…AndConverges". A case where the re-GET returns the already-updated CRD (and asserts a single update) would cover the realistic race-loser path, plus the re-GET→fresh-ResourceVersion retry mechanic. -
Error/edge branches of
crdUpdateWithNewVersionare untested: version-not-found (newVersion == nil), GET error inside the retry loop, and non-conflict / exhausted-retry terminal error.
Both are missing-coverage items only — nothing is broken. They can land in a follow-up so this PR isn't held up.
Backport from
maintov0.35resolves ENGCP-939
Original PR Nr.: #3993
Backported Commits: